feat: initialize Next.js app in /web directory#1
Merged
Conversation
Migrated all Next.js portions from velly-prototype including: - App Router pages and API routes - Components (Layout, RightPanel, etc.) - Database migrations and lib - GCP integration (Compute, Storage) - Anthropic AI integration - Editor templates - GitHub Actions workflows - Tailwind CSS v4 setup Ready for deployment on Vercel.
- Removed vercel.json - Removed @vercel/functions dependency - Removed waitUntil usage (now awaits directly) - Updated APP_URL fallbacks to localhost - Removed vercel.svg - Updated README
- Replaced @neondatabase/serverless with drizzle-orm + postgres - Added Drizzle schema (src/lib/schema.ts) with typed tables - Added drizzle.config.ts for migrations - Updated db.ts with both Drizzle and raw SQL compatibility - Removed old SQL migration files (now using Drizzle) - Updated GCS prefix from velly-prototype to vellum-assistant - Updated README and .env.example - Updated package.json scripts for Drizzle
Reorganized into monorepo structure: - /web - Next.js application - /editor-templates - Agent editor templates - / - Root config and CI
vargas-jr
Bot
changed the title
dvargasfuertes
approved these changes
Feb 7, 2026
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+36
to
+39
| // Legacy compatibility - getDb returns the drizzle instance | ||
| export function getDb() { | ||
| return db; | ||
| } |
Contributor
There was a problem hiding this comment.
🔴 Duplicate getDb() function definitions return different types, breaking all raw SQL callers
The db.ts file defines getDb() twice: once at line 22 returning the raw postgres SQL client, and again at line 37 returning the Drizzle ORM instance. The second definition shadows the first.
Root Cause and Impact
The first getDb() (line 22-24) returns the raw sql postgres client that supports template tag queries like sql\SELECT * FROM agents`. The second getDb()(line 37-39) returns the Drizzledb` instance, which has a completely different API.
Every API route in the codebase calls getDb() and uses it with template tag syntax, e.g.:
const sql = getDb();
const result = await sql`SELECT * FROM agents WHERE id = ${id}`;If the second definition takes precedence (as it would in JS), the Drizzle instance does NOT support template tag queries, causing all these API routes to fail at runtime. This affects web/src/app/api/agents/route.ts:75, web/src/app/api/agents/[id]/route.ts:11, and virtually every other API route.
Impact: All API endpoints that use getDb() with raw SQL template tags will crash at runtime.
Suggested change
| // Legacy compatibility - getDb returns the drizzle instance | |
| export function getDb() { | |
| return db; | |
| } | |
| // Legacy compatibility - getDb returns the raw SQL client | |
| // (duplicate removed - see line 22) | |
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+82
to
+98
| for (const line of lines) { | ||
| if (line.startsWith("event: ")) { | ||
| const eventType = line.slice(7); | ||
| const dataLineIndex = lines.indexOf(line) + 1; | ||
| const dataLine = lines[dataLineIndex]; | ||
| if (dataLine?.startsWith("data: ")) { | ||
| const data = JSON.parse(dataLine.slice(6)); | ||
| if (eventType === "progress") { | ||
| setProgressMessage(data.message); | ||
| } else if (eventType === "complete") { | ||
| router.push(`/agents/${data.agent.id}`); | ||
| return; | ||
| } else if (eventType === "error") { | ||
| throw new Error(data.message); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🔴 SSE parser uses indexOf on iterated array, causing wrong data line lookup for duplicate event types
The SSE parsing logic in handleCreateAgent uses lines.indexOf(line) to find the index of the current event line, then looks up the next line as the data line. indexOf returns the index of the first occurrence, not the current one.
Detailed Explanation
At web/src/app/agents/page.tsx:85, the code does:
const dataLineIndex = lines.indexOf(line) + 1;
const dataLine = lines[dataLineIndex];When the SSE stream contains multiple events of the same type (e.g., multiple event: progress lines), lines.indexOf(line) always returns the index of the first event: progress line in the lines array, not the current one being iterated. This means subsequent progress events will read the data from the first progress event's data line instead of their own.
For example, if lines contains:
["event: progress", "data: {step:naming}", "", "event: progress", "data: {step:upload}", ""]
When iterating over the second event: progress (index 3), indexOf returns 0, so dataLineIndex = 1, reading {step:naming} instead of {step:upload}.
Impact: Progress messages during agent creation may show stale/incorrect status, and the "complete" event could be missed if a prior event line has the same text.
Suggested change
| for (const line of lines) { | |
| if (line.startsWith("event: ")) { | |
| const eventType = line.slice(7); | |
| const dataLineIndex = lines.indexOf(line) + 1; | |
| const dataLine = lines[dataLineIndex]; | |
| if (dataLine?.startsWith("data: ")) { | |
| const data = JSON.parse(dataLine.slice(6)); | |
| if (eventType === "progress") { | |
| setProgressMessage(data.message); | |
| } else if (eventType === "complete") { | |
| router.push(`/agents/${data.agent.id}`); | |
| return; | |
| } else if (eventType === "error") { | |
| throw new Error(data.message); | |
| } | |
| } | |
| } | |
| let currentEventType: string | null = null; | |
| for (const line of lines) { | |
| if (line.startsWith("event: ")) { | |
| currentEventType = line.slice(7); | |
| } else if (line.startsWith("data: ") && currentEventType) { | |
| const data = JSON.parse(line.slice(6)); | |
| if (currentEventType === "progress") { | |
| setProgressMessage(data.message); | |
| } else if (currentEventType === "complete") { | |
| router.push(`/agents/${data.agent.id}`); | |
| return; | |
| } else if (currentEventType === "error") { | |
| throw new Error(data.message); | |
| } | |
| currentEventType = null; | |
| } else if (line === "") { | |
| currentEventType = null; | |
| } | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+69
to
+75
| await createChatMessage({ | ||
| agent_id: agentId, | ||
| role: "assistant", | ||
| content: outboxMsg.content, | ||
| status: "delivered", | ||
| gcs_message_id: outboxMsg.id, | ||
| }); |
Contributor
There was a problem hiding this comment.
🔴 createChatMessage called with snake_case keys but Drizzle schema expects camelCase
The createChatMessage function uses Drizzle ORM's insert().values() which expects keys matching the schema's camelCase property names, but callers pass snake_case keys.
Root Cause and Impact
The Drizzle schema at web/src/lib/schema.ts:19 defines agentId (camelCase) mapped to column agent_id, and gcsMessageId mapped to gcs_message_id. The NewChatMessage type inferred from this schema expects camelCase keys.
However, at web/src/app/api/agents/[id]/messages/route.ts:69-75 and route.ts:188-194, the callers pass:
await createChatMessage({
agent_id: agentId, // should be agentId
role: "assistant",
content: outboxMsg.content,
status: "delivered",
gcs_message_id: outboxMsg.id, // should be gcsMessageId
});Drizzle will not recognize agent_id or gcs_message_id as valid column references. The agent_id column is NOT NULL, so the insert will either fail with a database constraint violation (missing required agent_id) or silently insert NULL values.
Impact: All message creation (both user messages and synced assistant outbox messages) will fail, breaking the chat functionality entirely.
Suggested change
| await createChatMessage({ | |
| agent_id: agentId, | |
| role: "assistant", | |
| content: outboxMsg.content, | |
| status: "delivered", | |
| gcs_message_id: outboxMsg.id, | |
| }); | |
| await createChatMessage({ | |
| agentId: agentId, | |
| role: "assistant", | |
| content: outboxMsg.content, | |
| status: "delivered", | |
| gcsMessageId: outboxMsg.id, | |
| }); |
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+188
to
+194
| const dbMessage = await createChatMessage({ | ||
| agent_id: agentId, | ||
| role: "user", | ||
| content, | ||
| status: "sent", | ||
| gcs_message_id: agentData.messageId, | ||
| }); |
Contributor
There was a problem hiding this comment.
🔴 Second createChatMessage call also uses snake_case keys
Same issue as BUG-0003 but for the user message creation path at line 188-194.
Root Cause
At web/src/app/api/agents/[id]/messages/route.ts:188-194:
const dbMessage = await createChatMessage({
agent_id: agentId, // should be agentId
role: "user",
content,
status: "sent",
gcs_message_id: agentData.messageId, // should be gcsMessageId
});This will fail for the same reason as BUG-0003 - Drizzle expects camelCase property names.
Impact: Sending messages to agents will fail.
Suggested change
| const dbMessage = await createChatMessage({ | |
| agent_id: agentId, | |
| role: "user", | |
| content, | |
| status: "sent", | |
| gcs_message_id: agentData.messageId, | |
| }); | |
| const dbMessage = await createChatMessage({ | |
| agentId: agentId, | |
| role: "user", | |
| content, | |
| status: "sent", | |
| gcsMessageId: agentData.messageId, | |
| }); |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| // Obfuscate .env file contents | ||
| const filename = path.split("/").pop() || ""; | ||
| if (filename === ".env" || filename.endsWith(".env")) { |
Contributor
There was a problem hiding this comment.
🔴 .env obfuscation bypass for files like .env.local, .env.production
The env file detection logic fails to catch common env file variants that also contain secrets.
Root Cause
At web/src/app/api/agents/[id]/cat/route.ts:60:
if (filename === ".env" || filename.endsWith(".env")) {This only matches files named exactly .env or files ending with .env. It does NOT match common variants like .env.local, .env.production, .env.development, etc., because those end with .local, .production, etc.
A user browsing the file system can read the full unobfuscated contents of .env.local or .env.production files, which typically contain the same sensitive secrets as .env.
Impact: Secret values in .env.* variant files are exposed in plaintext through the file viewer API.
Suggested change
| if (filename === ".env" || filename.endsWith(".env")) { | |
| if (filename === ".env" || filename.startsWith(".env.") || filename.endsWith(".env")) { |
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+68
to
+71
| if (line.startsWith('event: ')) { | ||
| const eventType = line.slice(7); | ||
| continue; | ||
| } |
Contributor
There was a problem hiding this comment.
🔴 FocusView SSE parser discards eventType due to continue statement, never processes events
The SSE parser in FocusView.tsx reads the event type but immediately continues, so the event type is never used to determine how to handle the data.
Root Cause
At web/src/components/AssistantInitialization/FocusView.tsx:68-71:
if (line.startsWith('event: ')) {
const eventType = line.slice(7);
continue; // <-- skips to next iteration
}The eventType variable is declared inside the if block and immediately discarded by continue. The subsequent if (line.startsWith('data: ')) block processes data lines but has no access to the event type. This means the parser cannot distinguish between progress, complete, and error events.
The code partially works because it checks data.step and data.agent properties to infer the event type, but it never detects error events properly. If the server sends an error event, the data line will be processed but the error message check at line 105 (data.message.includes('error')) only logs it - it doesn't set the error state or stop processing.
Impact: Error events from the agent creation SSE stream are silently ignored. The user sees a perpetual loading state instead of an error message.
Prompt for agents
In web/src/components/AssistantInitialization/FocusView.tsx, the SSE parser at lines 67-111 needs to be restructured to properly track the event type across lines. Currently, the event type is read at line 69 but immediately discarded by the continue statement at line 70. The fix should:
1. Declare a variable outside the for loop (e.g., let currentEventType: string | null = null) to track the current event type.
2. When an event: line is encountered, store the event type in that variable (don't continue).
3. When a data: line is encountered, check currentEventType to determine how to handle it.
4. For 'error' events, call setError(data.message) to show the error UI.
5. For 'complete' events, extract the agent ID.
6. Reset currentEventType after processing a data line or on empty lines.
Was this helpful? React with 👍 or 👎 to provide feedback.
This was referenced Feb 8, 2026
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
Address remaining low-severity issues for production-ready shared library: 🔧 API Completeness (Issue #1): - Add public initializers to 17 remaining Encodable structs: • Skills: SkillsDisableMessage, SkillsConfigureMessage, SkillsInstallMessage, SkillsUninstallMessage, SkillsUpdateMessage, SkillsCheckUpdatesMessage, SkillsSearchMessage, SkillsInspectMessage • Apps: AppsListRequestMessage, SharedAppsListRequestMessage, SharedAppDeleteRequestMessage, BundleAppRequestMessage, OpenBundleMessage • Session: SessionListRequestMessage, HistoryRequestMessage • Signing: SignBundlePayloadResponseMessage, GetSigningIdentityResponseMessage - Ensures consistent public API surface for future iOS code 🛡️ Platform Safety (Issue #4): - Add #else fallback in DaemonClient.connect() with #error directive - Prevents silent compilation failures on unsupported platforms (visionOS, watchOS) - Clear compile-time error: "DaemonClient is only supported on macOS and iOS" 📝 Documentation (Issue #5): - Update DaemonClient doc comment to describe both connection types: • macOS: Unix domain socket at ~/.vellum/vellum.sock • iOS: TCP to configurable hostname:port via UserDefaults - Accurately reflects shared library's cross-platform design ✅ Build: passes in 3.70s ✅ All structs now have consistent public init coverage ✅ Future-proof against unsupported platform builds Note: iOS signing request handling (Issue #3) remains logged-only since response messages lack error fields. Proper fix requires daemon-side changes to avoid sending these requests to iOS clients. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
…riction Issue #5 (Low severity): - Make SendError enum public for cross-module error handling - Make errorDescription public for LocalizedError protocol conformance - Enables consumers to pattern-match on specific error cases (e.g., .notConnected) Issue #4 (Low severity - documentation): - Add comment clarifying VellumAssistantLib is macOS-only - Links macOS-specific frameworks (AppKit, ScreenCaptureKit, etc.) - iOS apps should depend only on VellumAssistantShared Note: Issue #1 (build script) is NOT a problem - SPM automatically searches parent directories for Package.swift. Build script works correctly as-is. ✅ Build: 4.65s Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
…ound Issue #1 (Consistency): - Add #else clause to signing operation switch cases in handleServerMessage - Ensures all platforms have explicit handling (logs error on unsupported platforms) - Complements the #error directive in connect() for defense in depth Issue #3 (Type inference fragility): - Replace .init shorthand with fully qualified ConfirmationAllowlistOption type - Consistent with other preview blocks in ToolConfirmationBubble.swift - Prevents breakage if type inference context changes Issue #7 (Build robustness): - Remove empty Resources directory and .process("Resources") declaration - Eliminates fragile .gitkeep workaround (accidental deletion would break builds) - VellumAssistantShared has no resources yet; can add back when needed Issues #2, #4, #5, #6 (Not actionable in this PR): - #2: VellumAssistantLib platform restriction documented via comment (SPM limitation) - #4: @mainactor deinit data race is pre-existing, not introduced by this PR - #5: iOS signing hang acknowledged, needs daemon-side fix - #6: JSON camelCase/snake_case mixed convention is existing protocol, not changing ✅ Build: 5.83s ✅ More robust against platform/type inference edge cases ✅ Removed unnecessary .gitkeep workaround Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
Issue #1 (🟡 Medium): Build script clean commands broken - .build directory moved from clients/macos/.build to clients/.build - build.sh clean and release pre-clean still referenced old location - Result: Clean commands did nothing, stale artifacts persisted Fix: Update both clean commands to use $SCRIPT_DIR/../.build - Line 64: Clean command now removes correct directory - Line 83: Release pre-clean now removes correct directory Issue #2 (🔴 High): MainActor.assumeIsolated crash risk in deinit - Previous fix (9231d63) used assumeIsolated which crashes if deinit runs on background thread - While assumeIsolated reveals lifecycle bugs, it's too aggressive for cleanup code that should never crash Fix: Revert to direct access pattern with detailed justification - Task.cancel(), NWConnection.cancel(), Continuation.finish() are all thread-safe - Data races on shouldReconnect and subscribers are benign in deinit (object is being destroyed, no other access possible) - Added comprehensive comment explaining the tradeoff Tradeoff analysis: ✅ No crashes during deallocation ✅ Clean command actually works now ✅ Release builds get proper artifact cleanup⚠️ Technically a data race, but benign for cleanup-only code⚠️ Won't trap on incorrect lifecycle (silent vs. loud failure) Devin review feedback addressed (both comments from 07:40-07:45) ✅ Build: 1.22s Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
…1821) * Create VellumAssistantShared library target for code reuse - Add multi-platform library target to Package.swift (macOS .v14, iOS .v17) - Move IPC layer (DaemonClient, IPCMessages) to shared library - Add platform-specific connection logic to DaemonClient: - macOS: Unix domain socket at ~/.vellum/vellum.sock - iOS: TCP endpoint (configured via UserDefaults) - Make all IPC types public with necessary initializers - Update macOS app to import VellumAssistantShared in all files using IPC - Add public initializers for Decodable messages used in tests Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Restructure directory layout for multi-platform support Move to flat structure at clients/ level: - Move Package.swift from clients/macos/ to clients/ - Move vellum-assistant-shared/ to clients/shared/ - Add .gitignore at clients/ level to exclude .build/ - Update Package.swift paths to reference shared/ and macos/ This prepares for iOS app in clients/ios/ without nesting everything under an Apple-specific directory. Final structure: clients/ Package.swift (Swift package for all Apple platforms) .gitignore (ignore .build, .swiftpm, etc) shared/ (VellumAssistantShared target) macos/ (macOS app) ios/ (future iOS app) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Address PR feedback: fix Resources directory and port overflow Fixes based on Devin and Codex review comments: 1. Create shared/Resources directory with .gitkeep - Package.swift declares resources: [.process("Resources")] - Git doesn't track empty directories, causing fresh clone failures - Added .gitkeep to ensure directory is tracked 2. Guard iOS daemon port conversion from UserDefaults overflow - Previous: Direct cast Int -> UInt16 can crash if value >65535 or negative - Fixed: Use UInt16(clamping:) with validation (0-65535 range) - Falls back to 8765 for invalid values - Prevents runtime crash from malformed persisted settings Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add missing public initializers and properties - Add public init to TaskSubmitMessage - Make SessionItem and HistoryMessageItem properties public - Make HistoryToolCallItem properties public Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix access control and iOS compatibility issues in shared IPC layer Address critical compilation issues and improve iOS platform handling: 🔴 Critical fixes: - Make SkillSummaryItem typealias public for cross-module access - Make UpdateInfo properties (name, installedVersion, latestVersion) public 🟡 Medium severity fixes: - Add public initializers to all Encodable IPC message structs: • PingMessage, SkillsEnableMessage • ConfirmationResponseMessage, SecretResponseMessage • AddTrustRuleMessage, TrustRulesListMessage • RemoveTrustRuleMessage, UpdateTrustRuleMessage - Add explicit error logging on iOS for unsupported signing operations (signBundlePayload, getSigningIdentity) instead of silently dropping These changes ensure the shared library has consistent public API surface for both macOS and iOS targets, and makes iOS behavior more transparent. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Complete public API consistency and add platform safety guards Address remaining low-severity issues for production-ready shared library: 🔧 API Completeness (Issue #1): - Add public initializers to 17 remaining Encodable structs: • Skills: SkillsDisableMessage, SkillsConfigureMessage, SkillsInstallMessage, SkillsUninstallMessage, SkillsUpdateMessage, SkillsCheckUpdatesMessage, SkillsSearchMessage, SkillsInspectMessage • Apps: AppsListRequestMessage, SharedAppsListRequestMessage, SharedAppDeleteRequestMessage, BundleAppRequestMessage, OpenBundleMessage • Session: SessionListRequestMessage, HistoryRequestMessage • Signing: SignBundlePayloadResponseMessage, GetSigningIdentityResponseMessage - Ensures consistent public API surface for future iOS code 🛡️ Platform Safety (Issue #4): - Add #else fallback in DaemonClient.connect() with #error directive - Prevents silent compilation failures on unsupported platforms (visionOS, watchOS) - Clear compile-time error: "DaemonClient is only supported on macOS and iOS" 📝 Documentation (Issue #5): - Update DaemonClient doc comment to describe both connection types: • macOS: Unix domain socket at ~/.vellum/vellum.sock • iOS: TCP to configurable hostname:port via UserDefaults - Accurately reflects shared library's cross-platform design ✅ Build: passes in 3.70s ✅ All structs now have consistent public init coverage ✅ Future-proof against unsupported platform builds Note: iOS signing request handling (Issue #3) remains logged-only since response messages lack error fields. Proper fix requires daemon-side changes to avoid sending these requests to iOS clients. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix SendError visibility and clarify VellumAssistantLib platform restriction Issue #5 (Low severity): - Make SendError enum public for cross-module error handling - Make errorDescription public for LocalizedError protocol conformance - Enables consumers to pattern-match on specific error cases (e.g., .notConnected) Issue #4 (Low severity - documentation): - Add comment clarifying VellumAssistantLib is macOS-only - Links macOS-specific frameworks (AppKit, ScreenCaptureKit, etc.) - iOS apps should depend only on VellumAssistantShared Note: Issue #1 (build script) is NOT a problem - SPM automatically searches parent directories for Package.swift. Build script works correctly as-is. ✅ Build: 4.65s Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix platform coverage consistency and remove fragile Resources workaround Issue #1 (Consistency): - Add #else clause to signing operation switch cases in handleServerMessage - Ensures all platforms have explicit handling (logs error on unsupported platforms) - Complements the #error directive in connect() for defense in depth Issue #3 (Type inference fragility): - Replace .init shorthand with fully qualified ConfirmationAllowlistOption type - Consistent with other preview blocks in ToolConfirmationBubble.swift - Prevents breakage if type inference context changes Issue #7 (Build robustness): - Remove empty Resources directory and .process("Resources") declaration - Eliminates fragile .gitkeep workaround (accidental deletion would break builds) - VellumAssistantShared has no resources yet; can add back when needed Issues #2, #4, #5, #6 (Not actionable in this PR): - #2: VellumAssistantLib platform restriction documented via comment (SPM limitation) - #4: @mainactor deinit data race is pre-existing, not introduced by this PR - #5: iOS signing hang acknowledged, needs daemon-side fix - #6: JSON camelCase/snake_case mixed convention is existing protocol, not changing ✅ Build: 5.83s ✅ More robust against platform/type inference edge cases ✅ Removed unnecessary .gitkeep workaround Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix @mainactor deinit data race in DaemonClient Issue: Swift concurrency bug (pre-existing, but now fixed) In Swift 5.9+, deinit on a @mainactor class is NOT guaranteed to run on the main actor. When the last reference to DaemonClient is dropped from a background thread, deinit could run on that thread, causing data races when accessing main-actor-isolated properties (shouldReconnect, connection, subscribers, etc.). Solution: Wrap deinit body in MainActor.assumeIsolated { } This ensures: - Safe access to main-actor-isolated state during cleanup - Trap with clear error if deinit somehow runs on wrong thread - Better than silent undefined behavior from data race Why fix in this PR: - Already heavily modifying DaemonClient for shared library - Real correctness issue (undefined behavior, potential crashes) - Simple fix with clear semantics - Makes shared library more robust for both macOS and iOS Reference: SE-0327 (Actors and Deinitializers) ✅ Build: 1.27s ✅ Eliminates undefined behavior from concurrent deinit Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix build script clean paths and revise deinit approach Issue #1 (🟡 Medium): Build script clean commands broken - .build directory moved from clients/macos/.build to clients/.build - build.sh clean and release pre-clean still referenced old location - Result: Clean commands did nothing, stale artifacts persisted Fix: Update both clean commands to use $SCRIPT_DIR/../.build - Line 64: Clean command now removes correct directory - Line 83: Release pre-clean now removes correct directory Issue #2 (🔴 High): MainActor.assumeIsolated crash risk in deinit - Previous fix (9231d63) used assumeIsolated which crashes if deinit runs on background thread - While assumeIsolated reveals lifecycle bugs, it's too aggressive for cleanup code that should never crash Fix: Revert to direct access pattern with detailed justification - Task.cancel(), NWConnection.cancel(), Continuation.finish() are all thread-safe - Data races on shouldReconnect and subscribers are benign in deinit (object is being destroyed, no other access possible) - Added comprehensive comment explaining the tradeoff Tradeoff analysis: ✅ No crashes during deallocation ✅ Clean command actually works now ✅ Release builds get proper artifact cleanup⚠️ Technically a data race, but benign for cleanup-only code⚠️ Won't trap on incorrect lifecycle (silent vs. loud failure) Devin review feedback addressed (both comments from 07:40-07:45) ✅ Build: 1.22s Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add explicit Network framework linking to VellumAssistantShared Issue: VellumAssistantShared imports Network (DaemonClient uses NWConnection) but doesn't explicitly link the Network framework. Currently works because VellumAssistantLib links Network, but future iOS targets depending only on VellumAssistantShared would fail. Fix: Add linkerSettings with Network framework to VellumAssistantShared target While SPM usually auto-links system frameworks on import, explicit linking ensures robust behavior when iOS app target depends only on the shared library (not on VellumAssistantLib which is macOS-only). ✅ Build: 16.92s (full rebuild) ✅ Explicit framework dependencies documented ✅ Future-proof for iOS app target Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add clients/ directory README for code organization documentation Created comprehensive README.md at clients/ level to document: - Directory structure and target organization - VellumAssistantShared vs VellumAssistantLib vs executable - Platform-specific vs shared code strategy - Build instructions and development guidelines - Migration context from single-platform structure - Known limitations (iOS TCP, signing, localhost) Helps developers and other agents understand: - Why Package.swift is at clients/ level - What code goes in shared/ vs macos/ vs ios/ - How to add new shared/platform-specific code - ~45-50% code reuse strategy Valuable for PR reviewers and future iOS development (PRs 2-13). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add imageData field to ToolResultMessage for protocol completeness The imageData field was added to ToolResultMessage in main (commit ea7c3d7) for browser screenshot support (PR #1864) but was inadvertently dropped during the migration to the shared library. This field enables the daemon to send base64-encoded image data from tool contentBlocks. While not currently consumed by the macOS client, adding it ensures the shared library's protocol definition matches the daemon's implementation. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Add missing public initializers to IPC message types Added public initializers to 7 Decodable message types that are used in test code. These types need explicit public initializers because Swift's auto-generated memberwise initializers are internal by default: - AssistantThinkingDeltaMessage - CuErrorMessage - CuCompleteMessage - CuActionMessage - GenerationHandoffMessage - MessageDequeuedMessage - GenerationCancelledMessage - ErrorMessage - MessageQueuedMessage Without these, tests that construct messages directly (rather than decoding from JSON) fail with "incorrect argument label" errors. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
5 tasks
ashleeradka
added a commit
that referenced
this pull request
Feb 14, 2026
Fix issues #1-3 from manual code review: 1. **Eliminate duplicated socket path resolution logic**: Extract resolveSocketPath() as a standalone function at module level. DaemonConfig.default now delegates to it (3 lines vs 14). DaemonClient.resolveSocketPath() also delegates for backward compatibility with existing tests. 2. **Handle empty hostname from UserDefaults**: Use .flatMap to treat empty string as nil, ensuring fallback to "localhost". Prevents connection attempts to empty hostname if user sets daemon_hostname="". 3. **Add @mainactor to iOS AppDelegate**: Consistent with macOS AppDelegate and required for strict concurrency checking since DaemonClient is @MainActor-isolated. Issues #4-6 acknowledged but not addressed in this PR: - #4: Silently ignored connection errors (acceptable for scaffold) - #5: iOS code under macos/ directory (can refactor later if needed) - #6: Platform-specific DaemonConfig (intentional design tradeoff) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 15, 2026
1. Add deleteThread() method for permanent thread deletion - Allows users to clean up hidden threads in drawer mode - Prevents indefinite accumulation of hidden threads 2. Fix hidden thread row click behavior - Removed .tag() from hidden thread rows - Only explicit restore button triggers restoration now 3. Optimize duplicate filter computation - Extract hiddenThreads to local variable - Avoid computing filter twice 4. Add delete button to hidden threads UI - Trash icon alongside restore button - Provides permanent cleanup option Fixes issues #2, #3, and #4 from review feedback. Issue #1 (sessionId data loss) was already fixed in previous commit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 15, 2026
1. ThreadModel.swift: Change sessionId from let to var - Fixes critical data loss bug where sessionId couldn't be persisted - Enables chat history restoration when threads are unhidden 2. VSplitView.swift: Improve split-view behavior - Change from overlay to side-by-side HStack layout - Panel now pushes content aside instead of covering it - Smoother spring animation (0.3s response, 0.8 damping) - Better padding (vertical + leading only) - Add clarifying comments These changes address the sessionId data loss issue (#1) and improve right panel UX per user feedback. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ashleeradka
added a commit
that referenced
this pull request
Feb 15, 2026
* Add thread drawer mode as alternative to tab layout Implements a NavigationSplitView-based drawer for accessing chat history, with a toggle in Display settings to switch between tab and drawer modes. ## Key Features - New "Show thread list drawer" toggle in Settings > Display - Drawer mode shows threads in a collapsible sidebar (NavigationSplitView) - Thread hiding system: X button hides threads instead of deleting - Hidden threads shown in collapsible "HIDDEN" section at bottom - Restore button (↺) to unhide threads in drawer mode - Consistent button behavior across both modes (icon-only toolbar buttons) ## Implementation Details - ThreadModel: Added `isHidden` property to track hidden threads - ThreadManager: Modified `closeThread()` to mark threads as hidden - ThreadManager: Added `showThread()` to restore hidden threads - MainWindowView: Conditional layout based on `useThreadDrawer` setting - ChatView: Reduced top padding in drawer mode for better spacing - Both modes now filter hidden threads from main list ## Technical Choices - Reuses existing NavigationSplitView component (no custom drawer) - Leverages ThreadManager's existing thread state management - Minimal code duplication between tab and drawer modes - Uses SwiftUI's native collapsible Section for hidden threads Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix thread drawer review issues - Sync selectedThreadId back from activeThreadId to keep sidebar selection in sync - Clean up ChatViewModels when threads are hidden to prevent memory leaks - Recreate ChatViewModels when hidden threads are restored - Use selectThread for visible threads instead of always calling showThread Addresses review feedback from devin-ai-integration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Address PR review feedback — thread management improvements 1. Add deleteThread() method for permanent thread deletion - Allows users to clean up hidden threads in drawer mode - Prevents indefinite accumulation of hidden threads 2. Fix hidden thread row click behavior - Removed .tag() from hidden thread rows - Only explicit restore button triggers restoration now 3. Optimize duplicate filter computation - Extract hiddenThreads to local variable - Avoid computing filter twice 4. Add delete button to hidden threads UI - Trash icon alongside restore button - Provides permanent cleanup option Fixes issues #2, #3, and #4 from review feedback. Issue #1 (sessionId data loss) was already fixed in previous commit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix sessionId immutability and improve VSplitView transitions 1. ThreadModel.swift: Change sessionId from let to var - Fixes critical data loss bug where sessionId couldn't be persisted - Enables chat history restoration when threads are unhidden 2. VSplitView.swift: Improve split-view behavior - Change from overlay to side-by-side HStack layout - Panel now pushes content aside instead of covering it - Smoother spring animation (0.3s response, 0.8 damping) - Better padding (vertical + leading only) - Add clarifying comments These changes address the sessionId data loss issue (#1) and improve right panel UX per user feedback. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Restore original tab mode delete behavior Devin correctly identified that closeThread() now hides threads in both modes, but tab mode originally deleted them permanently. This is a behavioral regression. Fix: Make closeThread() conditional based on useThreadDrawer setting: - Tab mode (useThreadDrawer = false): Delete threads permanently (original) - Drawer mode (useThreadDrawer = true): Hide threads (new feature) This preserves the original tab behavior while enabling the new drawer restoration feature only where it's accessible via UI. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Revert to original tab behavior - remove hide/restore feature Remove all hidden thread functionality to match original behavior: - Removed isHidden field from ThreadModel - Removed showThread() and deleteThread() methods from ThreadManager - Restored original simple closeThread() that removes from memory - Removed HIDDEN section from drawer mode UI - Removed isHidden filters from thread lists Original behavior preserved: - X button removes thread from memory - Threads restored on app restart from daemon sessions (up to 5 most recent) - Both tab and drawer modes now have identical thread lifecycle This simplifies the implementation and matches the original behavior exactly. Hide/restore can be added as a separate feature later. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix toolbar flash and address Devin review issues 1. Disable toolbar animations during NavigationSplitView transitions - Add .transaction modifier to disable animations - Wrap columnVisibility changes in withTransaction - Prevents toolbar flash when toggling drawer 2. Hide X button on last thread in drawer mode - Conditionally show close button only when threads.count > 1 - Matches tab mode behavior (ThreadTabBar) - Prevents non-functional button clicks 3. Fix VSplitView trailing padding - Change from .padding(.leading) to .padding(.horizontal) - Ensures right-side rounded corners aren't clipped - Provides proper spacing on both sides Addresses Devin review issues from commit c4cb938. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Enable drawer slide animation while preventing toolbar flash Changed approach to fix toolbar flash: - Removed broad .transaction modifier that disabled all animations - Added .animation(nil, value: columnVisibility) to toolbar HStack only - NavigationSplitView sidebar now slides in/out smoothly - Toolbar no longer flashes during sidebar transitions This gives us the best of both worlds: smooth drawer animation and stable toolbar positioning. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix root cause of toolbar flash - disable automatic sidebar toggle The "extra button" flash was caused by NavigationSplitView automatically adding a native sidebar toggle button to the toolbar. When the sidebar opened/closed, macOS would reposition/show/hide this button, causing our toolbar buttons to shift and flash. Fix: Add .toolbar(removing: .sidebarToggle) to disable the automatic sidebar toggle button that NavigationSplitView adds by default. Also removed the .animation(nil) workaround since we're now fixing the root cause instead of hiding the symptom. This is the proper fix - no animation suppression needed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Style drawer mode panels and fix layout animations - Add panel styling to chat content (rounded corners, consistent background) - Remove top padding from panels so they extend to button bar - Add smooth spring animation to HStack to prevent jumping when drawer opens/closes - Match ThreadTabBar layout pattern with VStack wrapper Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix drawer reopening and VSplitView padding issues - Reopen thread drawer when closing right-side panel (fixes drawer staying hidden) - Revert VSplitView padding to exclude top padding (panels flush with button bar) Addresses Devin review feedback on PR #2150 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Clean up code quality issues - Remove duplicate .ignoresSafeArea(edges: .top) in drawer mode - Merge duplicate .onAppear blocks into single initialization - Add comment explaining 78pt padding for traffic light buttons - Change ThreadModel.sessionId back to let (no longer mutated) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix shared component regressions and styling issues - Make chat panel styling conditional on drawer mode (fixes tab mode regression) - Revert VSplitView animation to VAnimation.standard (shared component fix) - Extract trafficLightPadding constant with detailed documentation Addresses critical review feedback: tab mode no longer gets unintended background/clipping, and VSplitView animation is consistent across all uses. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix drawer not preserving closed state when panel closes When Settings (or any panel) was opened with drawer already closed, closing the panel would incorrectly reopen the drawer. Now tracks previous drawer state and only reopens if it was open before. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Remove automatic drawer/panel linking behavior Drawer and panels now only open/close via manual toggle - no automatic closing of drawer when panel opens, no automatic reopening. Both are completely independent. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix thread item close button gesture conflict Restructured threadItem to use Button-based approach like ThreadTab, with close button in overlay instead of nested in HStack. This prevents onTapGesture from intercepting close button clicks. Fixes Devin review feedback. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 task
This was referenced Feb 17, 2026
This was referenced Feb 19, 2026
This was referenced May 19, 2026
Merged
7 tasks
This was referenced May 20, 2026
2 tasks
This was referenced May 22, 2026
This was referenced May 27, 2026
5 tasks
dvargasfuertes
pushed a commit
that referenced
this pull request
May 29, 2026
… on #32583) (#32596) Two pieces of Codex feedback on #32583. One was a legitimate bug, the other a misread of React reconciliation that this PR locks in as a regression test. ## Codex #1 — Legitimate bug, fixed The merged code applies `minHeight: viewportMinHeight` to the latest-edge wrapper whenever `renderAvatar` is present, even when `partition.anchorMessage` is null (assistant-only history: recovered conversations whose user message was lost, onboarding before the first submit). `useViewportMinHeight` reports the scroll container's `clientHeight` ≈ a full viewport. `refactor(web): scroll to bottom on transcript container DOM attach (#32239)` snaps the scroll to bottom on conversation switch. With no anchor at the top of the spacer, the bottom-pinned viewport lands on blank space + the avatar — the actual latest assistant message sits one viewport above and is invisible until the user scrolls up. Fix: gate both `minHeight: viewportMinHeight` and the `flex-1` spacer on `partition.anchorMessage`. Without an anchor, the avatar renders inline directly below the last history item. ## Codex #2 — Misread, locked in as a regression test Codex claimed that inserting `<LatestTurnRow>` at slot 0 (was `false`) on the no-anchor → anchor transition would cause React to reconcile the following `<div>` siblings by index, reusing the current avatar wrapper as the spacer and remounting `ChatAvatar` — replaying the entrance-spring animation #32583 is trying to avoid. Empirically verified that this does not happen. React's `reconcileChildrenArray` tracks `fiber.index` (the previous render's position in the children array, INCLUDING `false`-skipped slots). When slot 0 transitions from `false` to `<LatestTurnRow>`, the existing avatar fiber at `fiber.index=2` still matches `newIdx=2` in the next render — same `<div>` type, fiber reused, `ChatAvatar` is not unmounted. Locked in with a DOM-lifecycle regression test that: 1. Renders the transcript with no anchor + a mount-tracking avatar. 2. Asserts mount=1, unmount=0. 3. Rerenders with the first user message inserted (no-anchor → anchor). 4. Asserts mount=1, unmount=0 (no remount). 5. Rerenders back to no-anchor. 6. Asserts mount=1, unmount=0 again. A sanity-check version of the test (forcing a conversationId change, which re-keys the scroll container) was used to verify the mount tracker actually detects remounts — confirmed mount=2/unmount=1 when remount really happens, so the assertions can fail. ## Tests - 13 transcript.test.tsx tests pass. - 3 new Codex #1 cases: no-anchor has no min-height; anchor has min-height; anchor-without-avatar still has min-height (regression guard for the original viewport-pinning behavior). - 1 new Codex #2 case: the mount-tracker test above. - Typecheck clean (`tsc --noEmit --skipLibCheck` in `apps/web`). - ESLint clean on touched files. - 4 pre-existing `transcript-message-body.test.tsx` failures in the directory-wide run are also present on `main` — caused by mock bleed between test files, not by this PR. Co-authored-by: vellum-apollo-bot[bot] <242025090+vellum-apollo-bot[bot]@users.noreply.github.com>
This was referenced May 29, 2026
6 tasks
sarahkatebyte
pushed a commit
that referenced
this pull request
May 29, 2026
Companion to platform commit vellum-ai/vellum-assistant-platform@6aa58423e, which removes the `can_delete_payment_method` field from `AutoTopUpConfigResponse` per Carson's comment #1 on PR #7781: > can_delete_payment_method is redundant if only driven off whether or > not user is on pro plan The rule reduces to `plan_id == PRO`, so a server-authored boolean was redundant — the frontend can read `subscription.plan_id` directly. Changes: - payment-methods-card.tsx: re-add `useQuery(organizationsBilling SubscriptionRetrieveOptions())`, derive `isPro` locally, gate the Remove button on `!isPro` instead of the deleted `config.can_delete_payment_method`. Two ConfirmDialog variants for non-Pro users (auto-top-up on vs off) stay in place. - auto-top-up-card.tsx: drop `can_delete_payment_method: true` from the `DISABLED_CONFIG` seed (field no longer exists on the type). - openapi-schemas/platform.yaml: re-synced from platform repo post-Carson-fix; field is now absent from `AutoTopUpConfigResponse`. - generated/api/types.gen.ts (regen via `bun run openapi-ts`): `can_delete_payment_method: boolean` removed from `AutoTopUpConfigResponse`. - payment-methods-card.test.tsx: rewrite the three render tests to mock the subscription query instead of `config.can_delete_payment_ method`. Same cases: Base + saved card → Change + Remove; Pro + saved card → Change only; no saved card → Add Card only. Backend enforcement (`AutoTopUpViewSet.remove_payment_method` + `PaymentMethodViewSet.destroy` both return `409` for Pro plans) stays in place as the authoritative safety net. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Initial Next.js setup extracted from velly-prototype, organized as a monorepo.
Structure
Changes from velly-prototype
/webdirectoryTech Stack
Getting Started
cd web npm install npm run dev